Skip to content

chore: refacto setup part 1 #603

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 25, 2021
Merged

chore: refacto setup part 1 #603

merged 7 commits into from
Sep 25, 2021

Conversation

kyazdani42
Copy link
Member

@kyazdani42 kyazdani42 commented Aug 31, 2021

refacto setup for code entrypoint
following options switched to boolean values as options to the setup function:

  • nvim_tree_disable_netrw -> disable_netrw
  • nvim_tree_hijack_netrw -> hijack_netrw
  • nvim_tree_auto_open -> open_on_setup
  • nvim_tree_auto_close -> auto_close
  • nvim_tree_tab_open -> tab_open
  • nvim-tree-update-cwd -> update_cwd
  • nvim_tree_hijack_cursor -> hijack_cursor
  • nvim_tree_system_open_command -> system_open.cmd
  • nvim_tree_system_open_command_args -> system_open.args
  • nvim_tree_follow -> update_focused_file.enable
  • nvim_tree_follow_update_path -> update_focused_file.update_cwd
    Also added new option update_focused_file.ignore_list which will
    ignore filepath or filetypes that matches one entry of the list when
    updating the path if update_cwd is true.

After a few try of refactoring the setup in one go, i decided to move options bit by bit to the setup because it was too too complexe to do it in one go.
I'll do a step by step refactoring to remove all the g: options.

I'll leave this open for a week or so, so user can test this before merging to make sure nothing was broken.

This PR should enable lazy loading. All the others g: options should be set BEFORE running nvim-tree.setup

TODO:

  • update documentation (missing docs file)
  • fix the man hijack issue.
  • write a deprecation function for options in the runtime

should fix: #515 #184 #93
and maybe more but i'm not sure

@gegoune
Copy link
Collaborator

gegoune commented Sep 3, 2021

Have noticed some slow downs, running with

tree.setup {
  auto_close = true,
  update_focused_file = {
    enable = true,
  },
}

gives rally slow startup times:

    191.036  000.164  000.164: sourcing /Users/<me>/.config/nvim/after/plugin/nvim-notify.lua
    310.851  119.705  119.705: sourcing /Users/<me>/.config/nvim/after/plugin/nvim-tree.lua
    317.488  006.489  006.489: sourcing /Users/<me>/.config/nvim/after/plugin/nvim-treesitter.lua

The same file with config() commented out:

255.593  046.949  046.949: sourcing /Users/<me>/.config/nvim/after/plugin/nvim-tree.lua

(which is still super long) but it does not properly initialise nvim-tree on that branch. So setup is mandatory now I assume?

With removed setup those are the lines that contribute to those ~50ms:

local tree = require 'nvim-tree'
local tree_callback = require('nvim-tree.config').nvim_tree_callback

@kyazdani42
Copy link
Member Author

@gegoune could not reproduce the slowdown, on basic folders (10 to 500 folder/files), it opens in under 20ms in my machine (i debugged the setup part specifically with vim.loop.hrtime and vim.loop.now to be sure, i've managed to get it running to 80ms to 100ms in /usr/lib which contains ~4000 entries, and it looks even faster than running exa so i'm suprised it's running so slow on your machine. Maybe this comes from some place else in the code ?

@gegoune
Copy link
Collaborator

gegoune commented Sep 3, 2021

Just to be sure, I am not talking about opening nvim-tree window but time it takes to run setup + couple of those requires when opening nvim. I have it set up in after/plugin/nvim-tree.lua file.

@kyazdani42
Copy link
Member Author

i understand what you said before, but i could definitely not reproduce, on my machine it's really fast, opening the tree or not during setup does not change thing much, because well:

setup took 15.944164ms
rendering took 0.222216ms
rendering took 0.155439ms
rendering took 0.301504ms
rendering took 0.760211ms
rendering took 0.412551ms
rendering took 0.716118ms
rendering took 0.367504ms

not sure where this happens, i'm going to make some more debugging

@kyazdani42
Copy link
Member Author

@gegoune could you try this profiler to see if nvim-tree really is that slow compared to other plugins ? If so, could you send me your complete config for nvim-tree for the setup ?

@kyazdani42 kyazdani42 force-pushed the chore/refacto-setup-1 branch 2 times, most recently from 343843f to 55f6342 Compare September 3, 2021 19:45
@gegoune
Copy link
Collaborator

gegoune commented Sep 4, 2021

I will try with the profiler in a minute, although I have no idea how to use it.

From your output, isn't 15ms still quite a lot? Does nvim-tree actually retrieves directory's content on setup? I don't think it should do anything other than configure itself and only read directory when its window gets opened.

@kyazdani42
Copy link
Member Author

it does initialize the tree with basic content. This code is quite old 😄

type: `boolean`
default: `true`

- |open_on_setup|: will automatically the tree when running setup if current
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed 'open' after 'automatically'.

type: `boolean`
default: `false`

- |tab_open|: opens the tree automatically when switching tabpage or opening a new
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open_on_setup and tab_open are very different, perhaps they should both start with open_ and for example be like open_on_startup|setup and open_on_tab?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's what i told mysefl while writing the docs, thanks for confirming this!

type: `boolean`
default: `false`

- |tab_open|: opens the tree automatically when switching tabpage or opening a new
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or even

auto_open = {
  startup = bool,
  tab = bool,
}

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they are not really related. auto_open will open on startup automatically, tab will only keep the window open as if it was the same screen. I guess i'd like to rename the tab_open one because it's not really explicit.

@gegoune
Copy link
Collaborator

gegoune commented Sep 9, 2021

Have also noticed that nvim-tree doesn't change directory (:pwd) to git repository. It used to work very nicely with ygm2/rooter.nvim and although rooter changes pwd correctly it is not reflected in nvim-tree. All directories all the way to opened file fill be unfold, but root of tree will be my home directory rather than repository's root.

Does not work properly:

  1. Open nvim in ~: nvim; without any arguments.
  2. Use ibhagwan/fzf-lua to open file with MRU (oldfiles).

File opens, but after opening nvim-tree its root is still in ~

Does work properly:

  1. Open nvim in ~: nvim file/in/first/repo repo2/some/other/file/.
  2. Open nvim-tree and switch between both buffers.

nvim-tree's root nicely updates to repositories root. Works as expected.

@ibhagwan, do you think it might be caused by fzf-lua somehow? Although if I open another file using fzf-lua's oldfiles after second scenario then directory in nvim-tree updates nicely.

after/plugin/nvim-tree.lua
local tree = require 'nvim-tree'
local tree_callback = require('nvim-tree.config').nvim_tree_callback

vim.g.nvim_tree_bindings = {
  { key = '<C-s>', cb = tree_callback 'split' },
  { key = '<C-x>', cb = '' },
  { key = 'h', cb = tree_callback 'close_node' },
  { key = { '<CR>', 'l' }, cb = tree_callback 'edit' },
}

require('nvim-tree.view').View.winopts.cursorline = true

tree.setup {
  auto_close = true,
  update_cwd = true,
  update_focused_file = {
    enable = true,
    update_cwd = true, -- ?
  },
}
plugin/nvim-tree.lua
local g = vim.g
local bar = ''

-- g.nvim_tree_auto_close = 1
-- g.nvim_tree_follow = 1
g.nvim_tree_git_hl = 1
g.nvim_tree_group_empty = 1
g.nvim_tree_highlight_opened_files = 2
-- g.nvim_tree_hijack_cursor = 0
g.nvim_tree_icons = {
  git = {
    unstaged = 'M',
    staged = 'S',
    unmerged = 'U',
    renamed = 'R',
    untracked = '?',
    deleted = 'D',
    ignored = 'I',
  },
  folder = {
    default = '+',
    open = '-',
    empty = '+',
    empty_open = '-',
    symlink = '+',
    symlink_open = '-',
  },
  lsp = {
    error = bar,
    warning = bar,
    info = bar,
    hint = bar,
  },
}
g.nvim_tree_ignore = { '.git', 'node_modules', '.cache' }
g.nvim_tree_lsp_diagnostics = 1
g.nvim_tree_root_folder_modifier = ':t:r'
g.nvim_tree_show_icons = {
  files = 0,
  folders = 1,
  git = 1,
}
g.nvim_tree_special_files = {}
g.nvim_tree_width = 40

@gegoune
Copy link
Collaborator

gegoune commented Sep 9, 2021

@kyazdani42 I have also tried to get profiler working but couldn't. Do you mind showing me how to do that when you get a sec, please?

@ibhagwan
Copy link

ibhagwan commented Sep 9, 2021

@ibhagwan, do you think it might be caused by fzf-lua somehow? Although if I open another file using fzf-lua's oldfiles after second scenario then directory in nvim-tree updates nicely.

@gegoune, I don't think so, all fzf-lua does when you open a selected file is :e <filepath>, why don't you try your first scenario's 2nd step as :e <a recently edited file outside of your cwd> and see if you can reproduce the issue without fzf-lua?

@gegoune
Copy link
Collaborator

gegoune commented Sep 9, 2021

Ah, of course, didn't think of testing it like that. Case you described above does not update nvim-tree's root, so it's not fzf-lua's problem, forgive me for mentioning you for no good reason.

@kyazdani42 kyazdani42 force-pushed the chore/refacto-setup-1 branch from ebfb6e4 to 9492936 Compare September 12, 2021 12:18
@kyazdani42
Copy link
Member Author

kyazdani42 commented Sep 12, 2021

@gegoune maybe this is caused by vim.cmd "au DirChanged * lua require'nvim-tree.lib'.change_dir(vim.loop.cwd())" ? what does vim.loop.cwd shows when you change dir ? I'm not sure vim.loop follows internal vim path.

EDIT: was the same before actually i don't think its related.

I'm not sure but i don't think your root cwd updates as expected ? Also i've noted that rooter.nvim implemented special logic for nvim-tree so that's why it might not be working anymore.

@gegoune
Copy link
Collaborator

gegoune commented Sep 12, 2021

Oh, maybe, do you see anything wrong with their logic?

Just checked vim.loop.cwd - it returns git repository's root dir, same as :pwd.

@kyazdani42
Copy link
Member Author

kyazdani42 commented Sep 12, 2021

just did some debugging, the DirChanged event does not seem to be called at all and i'm not sure why, although vim directory was changed as expected. Not sure if this is a bug within neovim or rooter.nvim.I'm going to check wether other dirchanged event are well sent to the other plugins i have and i'll be back here with more infos.
Edit: so this seems to be a problem with rooter or neovim indeed, because even with a global vim.cmd[[au DirChanged * lua print('hello')]] at the start of init.lua does not trigger the dirchanged with vim-rooter. Not sure if this is rooter related or neovim related though.

@gegoune
Copy link
Collaborator

gegoune commented Sep 12, 2021

I have tried replacing rooter with ahmedkhalf/project.nvim and it doesn't work in the same way so probably rooter isn't at blame here.

@kyazdani42
Copy link
Member Author

Did not understand your last comment. If you mean it doesnt work in the same way, does this mean it fails in the same way ?
Also just tried the DirChanged disabling nvim-tree, and it still does not trigger the event so i think it comes from something else.

Ok, just found out... the rooter.nvim only run cd when g:loaded_tree is 1. So no change dir events are set otherwise, i think it might be a plugin issue, a rooter should change the base dir no matter what.
As for project.nvim i'm not sure.
Did you try https://github.com/airblade/vim-rooter ?

@gegoune
Copy link
Collaborator

gegoune commented Sep 12, 2021

Yes, sorry, meant exactly what you assumed, that it fails the same way.

Didn't try vim-rooter, might have some time later tonight.

But if rooter.nvim requires that var and project.nvim also doesn't work does it mean that there is some other issue at play here since rooter.ncim has no loaded_tree string anywhere in its code.

@kyazdani42
Copy link
Member Author

ok so, tested with a very minimal init.lua:

vim.cmd "set rtp+=~/project.nvim"
require'project_nvim'.setup {
  manual_mode = false,
  patterns = { ".git", "_darcs", ".hg", ".bzr", ".svn", "Makefile", "package.json" },
  silent_chdir = false,
}

vim.cmd 'au DirChanged * lua print(2)'

and it does change the directory as expected, but does not trigger the DirChanged event. I think it will be the same for every plugin then. This is definitely an neovim issue.

@kyazdani42
Copy link
Member Author

kyazdani42 commented Sep 12, 2021

or maybe because autocmd cannot be nested ?
EDIT: ok so i managed to fix this by adding a ++nested in project.nvim codebase.
@ahmedkhalf, is this something you wish to add ? @ygm2 also tagging

@ahmedkhalf
Copy link

ahmedkhalf commented Sep 18, 2021

Fixed, sorry for the long wait.

refacto setup for code entrypoint
following options switched boolean values as options to the setup function:
- `nvim_tree_disable_netrw` -> `disable_netrw`
- `nvim_tree_hijack_netrw` -> `hijack_netrw`
- `nvim_tree_auto_open` -> `open_on_setup`
- `nvim_tree_auto_close` -> `auto_close`
- `nvim_tree_tab_open` -> `tab_open`
- `nvim-tree-update-cwd` -> `update_cwd`
- `nvim_tree_hijack_cursor` -> `hijack_cursor`
- `nvim_tree_system_open_command` -> `system_open.cmd`
- `nvim_tree_system_open_command_args` -> `system_open.args`
- `nvim_tree_follow` -> `update_focused_file.enable`
- `nvim_tree_follow_update_path` -> `update_focused_file.update_cwd`
Also added new option `update_focused_file.ignore_list` which will
ignore filepath or filetypes that matches one entry of the list when
updating the path if update_cwd is true.
@kyazdani42 kyazdani42 force-pushed the chore/refacto-setup-1 branch from 9492936 to 83a238a Compare September 25, 2021 14:43
@kyazdani42 kyazdani42 merged commit a864b80 into master Sep 25, 2021
jim-fx pushed a commit to jim-fx/nvim-tree.lua that referenced this pull request Sep 28, 2021
* chore: refacto setup part 1

refacto setup for code entrypoint
following options switched boolean values as options to the setup function:
- `nvim_tree_disable_netrw` -> `disable_netrw`
- `nvim_tree_hijack_netrw` -> `hijack_netrw`
- `nvim_tree_auto_open` -> `open_on_setup`
- `nvim_tree_auto_close` -> `auto_close`
- `nvim_tree_tab_open` -> `tab_open`
- `nvim-tree-update-cwd` -> `update_cwd`
- `nvim_tree_hijack_cursor` -> `hijack_cursor`
- `nvim_tree_system_open_command` -> `system_open.cmd`
- `nvim_tree_system_open_command_args` -> `system_open.args`
- `nvim_tree_follow` -> `update_focused_file.enable`
- `nvim_tree_follow_update_path` -> `update_focused_file.update_cwd`
Also added new option `update_focused_file.ignore_list` which will
ignore filepath or filetypes that matches one entry of the list when
updating the path if update_cwd is true.

* add deprecation warning

* update readme

* schedule on enter to avoid running before vim first buffer has loaded

* update docs

* correct typo

* rename tab open -> open on tab
@kyazdani42 kyazdani42 deleted the chore/refacto-setup-1 branch October 9, 2021 09:10
Almo7aya pushed a commit to Almo7aya/nvim-tree.lua that referenced this pull request Oct 11, 2022
* chore: refacto setup part 1

refacto setup for code entrypoint
following options switched boolean values as options to the setup function:
- `nvim_tree_disable_netrw` -> `disable_netrw`
- `nvim_tree_hijack_netrw` -> `hijack_netrw`
- `nvim_tree_auto_open` -> `open_on_setup`
- `nvim_tree_auto_close` -> `auto_close`
- `nvim_tree_tab_open` -> `tab_open`
- `nvim-tree-update-cwd` -> `update_cwd`
- `nvim_tree_hijack_cursor` -> `hijack_cursor`
- `nvim_tree_system_open_command` -> `system_open.cmd`
- `nvim_tree_system_open_command_args` -> `system_open.args`
- `nvim_tree_follow` -> `update_focused_file.enable`
- `nvim_tree_follow_update_path` -> `update_focused_file.update_cwd`
Also added new option `update_focused_file.ignore_list` which will
ignore filepath or filetypes that matches one entry of the list when
updating the path if update_cwd is true.

* add deprecation warning

* update readme

* schedule on enter to avoid running before vim first buffer has loaded

* update docs

* correct typo

* rename tab open -> open on tab
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid window id when trying to open nvimtree on same file multiple times or sometimes on the first time.
4 participants